-
Notifications
You must be signed in to change notification settings - Fork 80
nn-models typings fix #840
Conversation
🚀 Deployed on https://deploy-preview-840--etna-docs.netlify.app |
Codecov Report
@@ Coverage Diff @@
## master #840 +/- ##
==========================================
- Coverage 84.64% 84.64% -0.01%
==========================================
Files 130 130
Lines 7450 7455 +5
==========================================
+ Hits 6306 6310 +4
- Misses 1144 1145 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
@@ -820,4 +820,70 @@ def get_model(self) -> "DeepBaseNet": | |||
return self.net | |||
|
|||
|
|||
BaseModel = Union[PerSegmentModel, PerSegmentPredictionIntervalModel, MultiSegmentModel, DeepBaseModel] | |||
class MultiSegmentPredictionIntervalModel(FitAbstractModel, PredictIntervalAbstractModel, BaseMixin): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we need just empty class without any implementation.
Downstream classes has implemented own logic already. Seems we can't share it
etna/models/nn/tft.py
Outdated
@@ -26,7 +26,7 @@ | |||
from pytorch_lightning import LightningModule | |||
|
|||
|
|||
class TFTModel(Model, PredictIntervalAbstractModel, _DeepCopyMixin): | |||
class TFTModel(Model, MultiSegmentPredictionIntervalModel, _DeepCopyMixin): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need Model here, I guess it's already in MultiSegmentPredictionIntervalModel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Before submitting (must do checklist)
Proposed Changes
Closing issues
closes #768